Skip to content

Conversation

compiler-errors
Copy link
Member

Get the arity correct when creating a FnSig type during tcx.fn_sig cycle recovery

Fixes #105152

@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2022

r? @nagisa

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 2, 2022
@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor

cjgillot commented Dec 2, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2022

📌 Commit 5809a05 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 2, 2022
…gillot

Properly synthesize `FnSig` value during cycle

Get the arity correct when creating a `FnSig` type during `tcx.fn_sig` cycle recovery

Fixes rust-lang#105152
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2022
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#105026 (v8a as default aarch64 target)
 - rust-lang#105033 (sparc-struct-abi: work around new tail-call optimization)
 - rust-lang#105144 (Document normalization methods `At::{normalize,query_normalize}`)
 - rust-lang#105155 (rustdoc: clean up help and settings button CSS)
 - rust-lang#105162 (Properly synthesize `FnSig` value during cycle)
 - rust-lang#105163 (Check lifetime param count in `collect_trait_impl_trait_tys`)
 - rust-lang#105185 (Move `normalize_fn_sig` to `TypeErrCtxt`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit babdf86 into rust-lang:master Dec 3, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 3, 2022
@compiler-errors compiler-errors deleted the fn-sig-arity branch August 11, 2023 20:01
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 18, 2024
Fix circular fn_sig queries to correct number of args for methods

Fixes rust-lang#130400. This was a [debug assert](https://github.com/rust-lang/rust/blob/28e8f01c2a2f33fb4214925a704e3223b372cad5/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L2557) added to some argument error reporting code in rust-lang#129320 which verified that the number of params (from the HIR) matched the `matched_inputs` which ultimately come from ty::FnSig. In the reduced test case:

```
fn foo(&mut self) -> _ {
    foo()
}
```

There is a circular dependency computing the ty::FnSig -- when trying to compute it, we try to figure out the return value, which again depends on this ty::FnSig. In rust-lang#105162, this was supported by short-circuiting the cycle by synthesizing a FnSig with error types for parameters. The [code in question](https://github.com/rust-lang/rust/pull/105162/files#diff-a65feec6bfffb19fbdc60a80becd1030c82a56c16b177182cd277478fdb04592R44) computes the number of parameters by taking the number of parameters from the hir::FnDecl and adding 1 if there is an implicit self parameter.

I might be missing a subtlety here, but AFAICT the adjustment for implicit self args is unnecessary and results in one too many args. For example, for this non-errorful code:

```
trait Foo {
    fn bar(&self) {}
}
```

The resulting hir::FnDecl and ty::FnSig both have the same number of inputs -- 1. So, this PR removes that adjustment and adds a test for the debug ICE.

r? `@compiler-errors`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2024
Rollup merge of rust-lang#130496 - jder:issue-130400, r=compiler-errors

Fix circular fn_sig queries to correct number of args for methods

Fixes rust-lang#130400. This was a [debug assert](https://github.com/rust-lang/rust/blob/28e8f01c2a2f33fb4214925a704e3223b372cad5/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L2557) added to some argument error reporting code in rust-lang#129320 which verified that the number of params (from the HIR) matched the `matched_inputs` which ultimately come from ty::FnSig. In the reduced test case:

```
fn foo(&mut self) -> _ {
    foo()
}
```

There is a circular dependency computing the ty::FnSig -- when trying to compute it, we try to figure out the return value, which again depends on this ty::FnSig. In rust-lang#105162, this was supported by short-circuiting the cycle by synthesizing a FnSig with error types for parameters. The [code in question](https://github.com/rust-lang/rust/pull/105162/files#diff-a65feec6bfffb19fbdc60a80becd1030c82a56c16b177182cd277478fdb04592R44) computes the number of parameters by taking the number of parameters from the hir::FnDecl and adding 1 if there is an implicit self parameter.

I might be missing a subtlety here, but AFAICT the adjustment for implicit self args is unnecessary and results in one too many args. For example, for this non-errorful code:

```
trait Foo {
    fn bar(&self) {}
}
```

The resulting hir::FnDecl and ty::FnSig both have the same number of inputs -- 1. So, this PR removes that adjustment and adds a test for the debug ICE.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE "index out of bounds" in xform_self_ty
7 participants